New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update: Add automated suggestion to radix
rule for parsing decimal numbers
#14291
Conversation
Hi @bmish!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
1 similar comment
Hi @bmish!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
Hi @bmish!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
Thanks for the PR! I support this enhancement 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
Is trailing comma case tested? |
Hi @bmish!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
Hi @bmish!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
@fisker good idea, added that test case. |
Edge case |
@fisker can you provide what it would be fixed to as well? |
I think a better fix should check the penultimate token, if it's a comma token, insert |
I'll give that a try. One other question, does it sound good for me to add suggestions for other common radix parameters?
|
Oh , I'm sorry, I fogot the second argument maybe already exists, maybe just check that fist argument |
I don't think that will be useful, if it's missing most time its's just forgot |
Hi @bmish!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
@fisker I added handling for the sequence expression. Good catch on that. For now, I'll keep only one suggestion. |
LGTM |
@eslint/eslint-tsc can I get another pair of eyes on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! LGTM
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[X] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What rule do you want to change?
radix
Does this change cause the rule to produce more or fewer warnings?
No.
How will the change be implemented? (New option, new default behavior, etc.)?
New automated suggestion.
What does the rule currently do for this code?
Does not provide an automated suggestion nor autofix.
What will the rule do after it's changed?
Provide an automated suggestion to change
to
because parsing decimal numbers is the most common use case.
What changes did you make? (Give an overview)
Added a suggestion fixer function and tests.
Is there anything you'd like reviewers to focus on?
No.